-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DOCS-1166][DOCS-880][DOCS-1167][DOCS-1168] Automation updates part 1 of 2 #1084
base: main
Are you sure you want to change the base?
Conversation
Deploying docs with
|
Latest commit: |
98a2a20
|
Status: | ✅ Deploy successful! |
Preview URL: | https://b8dcfb42.docodile.pages.dev |
Branch Preview URL: | https://automation-updates.docodile.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added some comments!
When we do add the docs for run metric events, they need a lot more explanation of how they work than what we have here - we should definitely meet at that time to walk through it
1. Provide a name for the webhook. | ||
1. Provide the endpoint URL for the webhook. | ||
1. If the webhook authenticates using an access token, set **Access token** to the name of the secret that contains the access token. When you configure an automation that uses this webhook, the access token will be available in the `$ACCESS_TOKEN` environment variable, and the HTTP header will have `Authorization: Bearer $ACCESS_TOKEN`. | ||
1. If the webhook validates its payload using a sensitive string, set **Secret** to the name of the secret that contains the sensitive string. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this doesn't really tell the user what happens to this value when we send the webhook. Is it sent in the body of the payload? is it available in the payload as $my_secret_name? is it sent in a header named secret?
So, TODO item for me: figure out what this does (unless @tonyyli-wandb knows off the top of your head?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know what you find out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe I've addressed this now.
The following tabs demonstrate example payloads based on common use cases. Within the examples they reference the following keys to refer to condition objects in the payload parameters: | ||
* `${event_type}` Refers to the type of event that triggered the action. | ||
* `${event_author}` Refers to the user that triggered the action. | ||
* `${artifact_version}` Refers to the specific artifact version that triggered the action. Passed as an artifact instance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to answer the question: what does it mean that something is 'passed as an artifact instance'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. This content is copied from where it lives in the current doc, not net new in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Partway through but have to step away, so wanted to leave any comments I have so far --
### Registry | ||
For a [registered model]({{< relref "/guides/models/registry/">}}), you can configure an automation to run on these events: | ||
|
||
- **Linking a new artifact to a registered model**: Test and validate new model candidates. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Since we're trying to move away from language that restricts artifacts to models alone:
- **Linking a new artifact to a registered model**: Test and validate new model candidates. | |
- **Linking a new artifact to a collection**: Test and validate new models, datasets, etc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: Also, this event is supported at the project scope
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, PTAL
For a [registered model]({{< relref "/guides/models/registry/">}}), you can configure an automation to run on these events: | ||
|
||
- **Linking a new artifact to a registered model**: Test and validate new model candidates. | ||
- **Adding a new alias to a version of the registered model**: Trigger a special step of your workflow when a new model version has a label or alias applied. For example, deploy a model when it has the `deploy` alias applied. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: This event is supported at both the "project" and "artifact collection" scope
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, PTAL
- **A new version of an artifact is created in a collection**: Apply recurring actions to each version of an artifact. For example, start a training job when a new dataset artifact version is created. | ||
- **An artifact alias is added**: Apply recurring actions when a specific alias is applied to an artifact version. For example, run a series of downstream processing steps when an artifact gains the `test-set-quality-check` alias. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: These events can be scoped at either the project
or artifact collection
level as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Co-authored-by: Stephen Sisk <[email protected]>
Specify the secrets you want to use for your webhook automation when you configure the webhook. See the [Configure a webhook]({{< relref "#configure-a-webhook" >}}) section for more information. | ||
|
||
{{% alert %}} | ||
Once you create a secret and grant the webhook access to it, you can use it in your automation's webhook payload by prefixing its name with with `$`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚫 [vale] reported by reviewdog 🐶
[Vale.Repetition] 'with' is repeated!
Reviewers: I have resolved the comments that I think I have addressed so far. Run metric automations will now be handled in a followup PR. Comments that remain open will be addressed soon! |
Co-authored-by: Tony Li <[email protected]>
I've implemented most of the open feedback from @ssisk @tonyyli-wandb , please take another look. Where I haven't said in the PR that it's addressed, I have not yet gotten to it (probably because I need to research it some more) or it will be addressed elsewhere. |
DOCS-1166 Update all possible links and references to old Model Registry
[DOCS-880]DOCS-1167 Consolidate project-scoped and model registry automations, new Slack notification flow, add redirects
Added @tonyyli-wandb @ibindlish for round 1 of technical review
Preview links for major changes: